fix: set surface event before SetFormat to prevent draw thread deadlock by winnerspiros · Pull Request #356 · winnerspiros/osu

winnerspiros · 2026-05-23T21:51:13Z

Problem

The reactive RGB565 guard in SurfaceChanged calls SetFormat(RGBA8888) which
triggers a synchronous surface teardown on the UI thread. The previous code
did surfaceEvent.Reset() + return, which caused the draw thread to block
indefinitely on surfaceEvent.Wait(5000). The native watchdog then fired
after 10s with 'no managed heartbeat ever observed'.

Fix

surfaceEvent.Set() BEFORE anything else — unblocks draw thread with current surface
NativeWatchdog.Heartbeat() before SetFormat — resets the 10s watchdog timer
holder.SetFormat() — triggers teardown (draw thread already has valid ref)

Files changed

osu.Android/OsuGameActivity.cs

Summary by CodeRabbit

Bug Fixes
- Resolved an issue causing unexpected application termination during rendering initialization on certain devices.
- Enhanced rendering stability through improved surface format change handling.

The previous approach (surfaceEvent.Reset() + return) caused the draw thread to block on surfaceEvent.Wait(5000) while SetFormat triggered a synchronous surface teardown. The draw thread never got a valid surface handle and the native watchdog fired. New approach: Set surfaceEvent BEFORE calling SetFormat so the draw thread can proceed with the current (soon-to-be-recreated) surface. The native watchdog is also ticked before SetFormat to buy time for the synchronous teardown. Order: surfaceEvent.Set() → NativeWatchdog.Heartbeat() → holder.SetFormat()

coderabbitai · 2026-05-23T21:51:38Z

Warning

Review limit reached

@winnerspiros, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 1 review/hour. Refill in 46 minutes and 33 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fbc06269-44b9-44ed-8cc6-601ebba7edf3

📥 Commits

Reviewing files that changed from the base of the PR and between 2dd8eeb and fb435de.

📒 Files selected for processing (1)

osu.Android/OsuGameActivity.cs

📝 Walkthrough

Walkthrough

The PR reorders the Vulkan RGB565→RGBA8888 surface format recovery path in SurfaceChanged. It now signals the draw thread to continue before triggering blocking surface teardown work, preventing watchdog timeout. The format-change log is moved earlier, and the immediate event reset after format request is removed, deferring reset to the normal lifecycle.

Changes

RGB565→RGBA8888 Recovery Event Sequencing

Layer / File(s)	Summary
Event signal reordering in surface format recovery `osu.Android/OsuGameActivity.cs`	`surfaceEvent` is signaled before format-change work begins, allowing the draw thread to proceed during UI-thread teardown; the format-change log is moved earlier and the immediate event reset is removed, delegating that to the normal `SurfaceDestroyed` lifecycle.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A draw thread waits by the surface gate,
The UI thread says "don't hesitate!"—
Set first, then tear, then reset with grace,
Watchdog stays happy, no timeout race. 🎮✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: set surface event before SetFormat to prevent draw thread deadlock' directly and accurately describes the main change: reordering surface event signaling before SetFormat to prevent draw thread blocking.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/surface-event-before-setformat

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Review feedback: setting surfaceEvent before SetFormat is dangerous because the draw thread can get a stale surface pointer that's being torn down. New approach: Set surfaceEvent AFTER SetFormat returns. By this point, the synchronous SurfaceDestroyed→SurfaceCreated→SurfaceChanged cycle has already completed and surfaceGlobalRef points to the new RGBA8888 surface. This preserves the invariant that surfaceEvent is only set when surfaceGlobalRef points to a safe-to-render surface. The NativeWatchdog.Heartbeat() is still called before SetFormat to reset the watchdog timer during the blocking teardown.

Copilot AI review requested due to automatic review settings May 23, 2026 21:51

Copilot started reviewing on behalf of winnerspiros May 23, 2026 21:51 View session

Copilot AI reviewed May 23, 2026

winnerspiros merged commit 9833352 into master May 23, 2026
15 of 23 checks passed

winnerspiros mentioned this pull request May 24, 2026

fix: reset VulkanUiWatchdog pong before SetFormat to prevent 7s kill #357

Merged

Navigation Menu

fix: set surface event before SetFormat to prevent draw thread deadlock - #356

fix: set surface event before SetFormat to prevent draw thread deadlock#356
winnerspiros merged 2 commits into
masterfrom
fix/surface-event-before-setformat

winnerspiros commented May 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 23, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

winnerspiros commented May 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Files changed

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

winnerspiros commented May 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 23, 2026 •

edited

Loading